-
Notifications
You must be signed in to change notification settings - Fork 9
chore: Parse identifiers for __ctHelpers usage rather than a naive string check #1896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
|
|
||
| // Throws if `__ctHelpers` was found as an Identifier | ||
| // in the source code. | ||
| function checkCTHelperVar(source: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, it's very cool that this is actually faster than the string check - i for sure would not have guessed that!
| ); | ||
| } | ||
| } | ||
| return ts.visitEachChild(node, visitor, undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
claude suggests that you could consider using ts.forEachChild(node, visitor) just to avoid having the potentially confusing undefined third parameter for the TransformationContext. i think it's probably good to have it this way because we use visitEachChild elsewhere and it keeps us consistent, but, just mentioning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think forEachChild works as well, changing the return value to ts.Node | undefined, though the iteration logic seems different, but looking more into it (tests pass with it however)
Summary by cubic
Detect the reserved __ctHelpers only when used as an identifier, instead of using a raw string check. This removes false positives from comments and similar names.